-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(promexporter): relayer balances #3016
Conversation
WalkthroughThe changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Exporter
participant API
participant Metrics
Client->>Exporter: Request metrics
Exporter->>API: Fetch relayer balances
API-->>Exporter: Provide balances
Exporter->>Metrics: Record relayer balances
Metrics-->>Exporter: Confirm recording
Exporter-->>Client: Return metrics
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3016 +/- ##
===================================================
- Coverage 24.93457% 24.86862% -0.06596%
===================================================
Files 800 801 +1
Lines 58080 58226 +146
Branches 82 82
===================================================
- Hits 14482 14480 -2
- Misses 42108 42255 +147
- Partials 1490 1491 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine-fe with Cloudflare Pages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Outside diff range, codebase verification and nitpick comments (2)
contrib/promexporter/exporters/relayer.go (1)
75-101
: Review offetchAllQuotes
function.The function correctly handles HTTP requests and JSON parsing. Ensure that error messages provide sufficient context for debugging, especially in production environments.
Consider adding more context to error messages to aid in debugging.
Tools
GitHub Check: codecov/patch
[warning] 75-79: contrib/promexporter/exporters/relayer.go#L75-L79
Added lines #L75 - L79 were not covered by tests
[warning] 81-87: contrib/promexporter/exporters/relayer.go#L81-L87
Added lines #L81 - L87 were not covered by tests
[warning] 89-92: contrib/promexporter/exporters/relayer.go#L89-L92
Added lines #L89 - L92 were not covered by tests
[warning] 94-98: contrib/promexporter/exporters/relayer.go#L94-L98
Added lines #L94 - L98 were not covered by tests
[warning] 100-100: contrib/promexporter/exporters/relayer.go#L100
Added line #L100 was not covered by testsethergo/go.mod (1)
TOML library is still referenced in the codebase.
The dependency
github.com/BurntSushi/toml
has been removed fromgo.mod
, but it is still referenced in the fileagents/internal/require.go
. Please review this file to ensure that the usage of the TOML library has been properly refactored or replaced.
- File:
agents/internal/require.go
- Reference:
github.com/BurntSushi/toml
Analysis chain
Line range hint
1-3
:
Verify removal of TOML library usage.The
github.com/BurntSushi/toml
dependency has been removed. Ensure that all instances of its usage have been refactored or replaced in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the TOML library is no longer used in the codebase. # Test: Search for any remaining references to the TOML library. Expect: No matches. rg --type go 'BurntSushi/toml'Length of output: 87
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
contrib/promexporter/go.sum
is excluded by!**/*.sum
contrib/screener-api/go.sum
is excluded by!**/*.sum
core/go.sum
is excluded by!**/*.sum
Files selected for processing (8)
- contrib/promexporter/config/config.go (2 hunks)
- contrib/promexporter/exporters/exporter.go (1 hunks)
- contrib/promexporter/exporters/otel.go (6 hunks)
- contrib/promexporter/exporters/otel_generated.go (1 hunks)
- contrib/promexporter/exporters/relayer.go (1 hunks)
- contrib/promexporter/go.mod (1 hunks)
- contrib/screener-api/go.mod (1 hunks)
- ethergo/go.mod (1 hunks)
Files skipped from review due to trivial changes (1)
- contrib/screener-api/go.mod
Additional context used
GitHub Check: codecov/patch
contrib/promexporter/exporters/relayer.go
[warning] 19-24: contrib/promexporter/exporters/relayer.go#L19-L24
Added lines #L19 - L24 were not covered by tests
[warning] 27-33: contrib/promexporter/exporters/relayer.go#L27-L33
Added lines #L27 - L33 were not covered by tests
[warning] 35-37: contrib/promexporter/exporters/relayer.go#L35-L37
Added lines #L35 - L37 were not covered by tests
[warning] 40-44: contrib/promexporter/exporters/relayer.go#L40-L44
Added lines #L40 - L44 were not covered by tests
[warning] 46-49: contrib/promexporter/exporters/relayer.go#L46-L49
Added lines #L46 - L49 were not covered by tests
[warning] 51-57: contrib/promexporter/exporters/relayer.go#L51-L57
Added lines #L51 - L57 were not covered by tests
[warning] 59-69: contrib/promexporter/exporters/relayer.go#L59-L69
Added lines #L59 - L69 were not covered by tests
[warning] 72-72: contrib/promexporter/exporters/relayer.go#L72
Added line #L72 was not covered by tests
[warning] 75-79: contrib/promexporter/exporters/relayer.go#L75-L79
Added lines #L75 - L79 were not covered by tests
[warning] 81-87: contrib/promexporter/exporters/relayer.go#L81-L87
Added lines #L81 - L87 were not covered by tests
[warning] 89-92: contrib/promexporter/exporters/relayer.go#L89-L92
Added lines #L89 - L92 were not covered by tests
[warning] 94-98: contrib/promexporter/exporters/relayer.go#L94-L98
Added lines #L94 - L98 were not covered by tests
[warning] 100-100: contrib/promexporter/exporters/relayer.go#L100
Added line #L100 was not covered by testscontrib/promexporter/exporters/exporter.go
[warning] 124-127: contrib/promexporter/exporters/exporter.go#L124-L127
Added lines #L124 - L127 were not covered by testscontrib/promexporter/exporters/otel.go
[warning] 66-73: contrib/promexporter/exporters/otel.go#L66-L73
Added lines #L66 - L73 were not covered by tests
[warning] 109-111: contrib/promexporter/exporters/otel.go#L109-L111
Added lines #L109 - L111 were not covered by tests
[warning] 143-145: contrib/promexporter/exporters/otel.go#L143-L145
Added lines #L143 - L145 were not covered by tests
[warning] 335-338: contrib/promexporter/exporters/otel.go#L335-L338
Added lines #L335 - L338 were not covered by tests
[warning] 344-347: contrib/promexporter/exporters/otel.go#L344-L347
Added lines #L344 - L347 were not covered by tests
[warning] 349-359: contrib/promexporter/exporters/otel.go#L349-L359
Added lines #L349 - L359 were not covered by tests
[warning] 361-361: contrib/promexporter/exporters/otel.go#L361
Added line #L361 was not covered by tests
[warning] 364-364: contrib/promexporter/exporters/otel.go#L364
Added line #L364 was not covered by tests
GitHub Check: Lint (contrib/promexporter)
contrib/promexporter/config/config.go
[failure] 35-35:
tag is not aligned , should be: default:"http://rfq-api.omnirpc.io/quotes" yaml:"rfq_api_url" (tagalign)
Additional comments not posted (4)
contrib/promexporter/exporters/otel_generated.go (1)
17-18
: Addition ofRecordRelayerBalance
method is appropriate.The new method
RecordRelayerBalance
enhances theiOtelRecorder
interface by allowing relayer balance tracking. Ensure that all implementations of this interface are updated accordingly.contrib/promexporter/exporters/relayer.go (1)
19-73
: Review offetchRelayerBalances
logic.The function efficiently fetches relayer balances using batch calls. Consider handling potential errors from
batchCalls
to ensure robustness. Additionally, verify thatchainIDToRelayers
is correctly populated and used.Tools
GitHub Check: codecov/patch
[warning] 19-24: contrib/promexporter/exporters/relayer.go#L19-L24
Added lines #L19 - L24 were not covered by tests
[warning] 27-33: contrib/promexporter/exporters/relayer.go#L27-L33
Added lines #L27 - L33 were not covered by tests
[warning] 35-37: contrib/promexporter/exporters/relayer.go#L35-L37
Added lines #L35 - L37 were not covered by tests
[warning] 40-44: contrib/promexporter/exporters/relayer.go#L40-L44
Added lines #L40 - L44 were not covered by tests
[warning] 46-49: contrib/promexporter/exporters/relayer.go#L46-L49
Added lines #L46 - L49 were not covered by tests
[warning] 51-57: contrib/promexporter/exporters/relayer.go#L51-L57
Added lines #L51 - L57 were not covered by tests
[warning] 59-69: contrib/promexporter/exporters/relayer.go#L59-L69
Added lines #L59 - L69 were not covered by tests
[warning] 72-72: contrib/promexporter/exporters/relayer.go#L72
Added line #L72 was not covered by testscontrib/promexporter/exporters/exporter.go (1)
124-127
: Review of error handling incollectMetrics
.The error handling for
fetchRelayerBalances
is well-integrated with the existing error collection mechanism. Ensure that all potential errors are captured and logged appropriately.Tools
GitHub Check: codecov/patch
[warning] 124-127: contrib/promexporter/exporters/exporter.go#L124-L127
Added lines #L124 - L127 were not covered by testscontrib/promexporter/go.mod (1)
39-39
: Verify the necessity of the new RFQ dependency.The addition of
github.com/synapsecns/sanguine/services/rfq v1.8.0
should be verified to ensure it is required and does not introduce unwanted dependencies.Verification successful
The RFQ dependency is necessary and widely used.
The
github.com/synapsecns/sanguine/services/rfq v1.8.0
dependency is extensively utilized across the codebase, including in services, APIs, and tests. This confirms its necessity and integration into the project's functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new RFQ dependency in the codebase. # Test: Search for the usage of the RFQ package. Expect: Occurrences of RFQ-related code. rg --type go 'rfq'Length of output: 53373
func (e *exporter) fetchRelayerBalances(ctx context.Context, url string) error { | ||
// Fetch relayer addresses | ||
quotes, err := e.fetchAllQuotes(ctx, url) | ||
if err != nil { | ||
return fmt.Errorf("could not fetch relayer addresses: %w", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding test coverage for fetchRelayerBalances
.
The function fetchRelayerBalances
lacks test coverage, as indicated by static analysis tools. Adding tests will help ensure its reliability and correctness.
Would you like assistance in creating tests for this function?
Tools
GitHub Check: codecov/patch
[warning] 19-24: contrib/promexporter/exporters/relayer.go#L19-L24
Added lines #L19 - L24 were not covered by tests
func (e *exporter) fetchAllQuotes(ctx context.Context, url string) ([]rfqAPIModel.GetQuoteResponse, error) { | ||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not get quotes: %w", err) | ||
} | ||
|
||
res, err := e.client.Do(req) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not get quotes: %w", err) | ||
} | ||
defer func() { | ||
_ = res.Body.Close() | ||
}() | ||
|
||
body, err := io.ReadAll(res.Body) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not read body: %w", err) | ||
} | ||
|
||
var quotes []rfqAPIModel.GetQuoteResponse | ||
err = json.Unmarshal(body, "es) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not unmarshal quotes: %w", err) | ||
} | ||
|
||
return quotes, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding test coverage for fetchAllQuotes
.
The function fetchAllQuotes
lacks test coverage, as indicated by static analysis tools. Adding tests will help ensure its reliability and correctness.
Would you like assistance in creating tests for this function?
Tools
GitHub Check: codecov/patch
[warning] 75-79: contrib/promexporter/exporters/relayer.go#L75-L79
Added lines #L75 - L79 were not covered by tests
[warning] 81-87: contrib/promexporter/exporters/relayer.go#L81-L87
Added lines #L81 - L87 were not covered by tests
[warning] 89-92: contrib/promexporter/exporters/relayer.go#L89-L92
Added lines #L89 - L92 were not covered by tests
[warning] 94-98: contrib/promexporter/exporters/relayer.go#L94-L98
Added lines #L94 - L98 were not covered by tests
[warning] 100-100: contrib/promexporter/exporters/relayer.go#L100
Added line #L100 was not covered by tests
type relayerMetadata struct { | ||
address common.Address | ||
balance float64 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for relayerMetadata
.
The new relayerMetadata
struct should be covered by tests to ensure its proper usage in the codebase.
Do you want me to generate test cases for this new struct or open a GitHub issue to track this task?
relayerBalance *hashmap.Map[int, []relayerMetadata] | ||
relayerBalanceGauge metric.Float64ObservableGauge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for relayerBalance
in otelRecorder
.
The relayerBalance
field in otelRecorder
should be covered by tests to ensure its functionality is working as expected.
Do you want me to generate test cases for this new field or open a GitHub issue to track this task?
if otr.relayerBalanceGauge, err = otr.meter.Float64ObservableGauge("relayer_balance"); err != nil { | ||
log.Warnf("failed to create relayerBalance gauge: %v", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for relayerBalanceGauge
.
The creation of relayerBalanceGauge
should be covered by tests to ensure it is initialized correctly.
Do you want me to generate test cases for this gauge or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 109-111: contrib/promexporter/exporters/otel.go#L109-L111
Added lines #L109 - L111 were not covered by tests
if _, err = otr.meter.RegisterCallback(otr.recordRelayerBalance, otr.relayerBalanceGauge); err != nil { | ||
log.Warnf("failed to register callback for relayer balance metrics: %v", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for recordRelayerBalance
callback registration.
The registration of the recordRelayerBalance
callback should be covered by tests to ensure it is functioning correctly.
Do you want me to generate test cases for this callback registration or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 143-145: contrib/promexporter/exporters/otel.go#L143-L145
Added lines #L143 - L145 were not covered by tests
func (o *otelRecorder) RecordRelayerBalance(chainID int, relayer relayerMetadata) { | ||
relayerBalances, _ := o.relayerBalance.Get(chainID) | ||
relayerBalances = append(relayerBalances, relayer) | ||
o.relayerBalance.Set(chainID, relayerBalances) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for RecordRelayerBalance
method.
The RecordRelayerBalance
method should be covered by tests to ensure it correctly records relayer balances.
Do you want me to generate test cases for this method or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 335-338: contrib/promexporter/exporters/otel.go#L335-L338
Added lines #L335 - L338 were not covered by tests
func (o *otelRecorder) recordRelayerBalance( | ||
_ context.Context, | ||
observer metric.Observer, | ||
) (err error) { | ||
if o.metrics == nil || o.relayerBalance == nil { | ||
return nil | ||
} | ||
|
||
o.relayerBalance.Range(func(chainID int, relayerBalances []relayerMetadata) bool { | ||
for _, relayer := range relayerBalances { | ||
observer.ObserveFloat64( | ||
o.relayerBalanceGauge, | ||
relayer.balance, | ||
metric.WithAttributes( | ||
attribute.Int(metrics.ChainID, chainID), | ||
attribute.String("relayer_address", relayer.address.String()), | ||
), | ||
) | ||
} | ||
|
||
return true | ||
}) | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for recordRelayerBalance
method.
The recordRelayerBalance
method should be covered by tests to ensure it correctly observes relayer balances.
Do you want me to generate test cases for this method or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 344-347: contrib/promexporter/exporters/otel.go#L344-L347
Added lines #L344 - L347 were not covered by tests
[warning] 349-359: contrib/promexporter/exporters/otel.go#L349-L359
Added lines #L349 - L359 were not covered by tests
[warning] 361-361: contrib/promexporter/exporters/otel.go#L361
Added line #L361 was not covered by tests
[warning] 364-364: contrib/promexporter/exporters/otel.go#L364
Added line #L364 was not covered by tests
|
||
if err := e.fetchRelayerBalances(ctx, e.cfg.RFQAPIUrl); err != nil { | ||
errs = append(errs, fmt.Errorf("could not fetch relayer balances: %w", err)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure test coverage for new error handling logic.
The error handling logic for fetching relayer balances in collectMetrics
lacks test coverage. Consider adding tests to verify its behavior under different scenarios.
Would you like assistance in creating tests for this error handling logic?
Tools
GitHub Check: codecov/patch
[warning] 124-127: contrib/promexporter/exporters/exporter.go#L124-L127
Added lines #L124 - L127 were not covered by tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request introduces functionality to fetch and record relayer balances across different chains in the Prometheus exporter. Key changes include:
- Added new
RFQAPIUrl
field inConfig
struct for interacting with the RFQ API - Implemented
fetchRelayerBalances
function inrelayer.go
to retrieve and record relayer balances - Extended
otelRecorder
to support recording relayer balance metrics - Integrated relayer balance fetching into the main metrics collection process
- Updated dependencies, including adding support for the Synapse RFQ service
The changes enhance the exporter's capabilities by providing insights into relayer balances across various chains, which can be useful for monitoring and analysis purposes.
11 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
func (o *otelRecorder) RecordRelayerBalance(chainID int, relayer relayerMetadata) { | ||
relayerBalances, _ := o.relayerBalance.Get(chainID) | ||
relayerBalances = append(relayerBalances, relayer) | ||
o.relayerBalance.Set(chainID, relayerBalances) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: This method might overwrite existing relayer data for the same chainID. Consider using a map with relayer address as key instead of a slice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explain more?
) | ||
} | ||
|
||
_ = e.batchCalls(ctx, client, callsForCurrentChainID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Ignoring the error returned by e.batchCalls may lead to silent failures. Consider handling or logging this error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its fine
defer func() { | ||
_ = res.Body.Close() | ||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using defer res.Body.Close()
instead of a defer function for simplicity.
@@ -39,7 +39,6 @@ require ( | |||
require ( | |||
dario.cat/mergo v1.0.0 // indirect | |||
github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Dependency on github.com/BurntSushi/toml removed. Verify if TOML parsing is still needed and replace with an alternative if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
This pull request enhances the Prometheus exporter's functionality to fetch and record relayer balances across different chains. The main changes focus on implementing the relayer balance fetching mechanism and integrating it into the existing metrics collection process.
Key updates include:
- Implemented
fetchRelayerBalances
function incontrib/promexporter/exporters/relayer.go
to retrieve relayer balances from multiple chains - Added
fetchAllQuotes
function inrelayer.go
to get quotes from the RFQ API - Integrated relayer balance fetching into
collectMetrics
function incontrib/promexporter/exporters/exporter.go
- Extended
otelRecorder
interface to includeRecordRelayerBalance
method for recording relayer metrics
These changes provide valuable insights into relayer balances across various chains, enhancing monitoring and analysis capabilities for the Sanguine project.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
This pull request further enhances the Prometheus exporter's functionality by adding support for fetching and recording relayer quotes, alongside the previously implemented relayer balance tracking.
Key updates:
- Added
FetchRelayerQuotes
function incontrib/promexporter/exporters/relayer.go
to retrieve quotes from the RFQ API - Integrated quote fetching into the
collectMetrics
function incontrib/promexporter/exporters/exporter.go
- Extended the
otelRecorder
interface withRecordRelayerQuote
method for recording quote metrics - Implemented error handling and logging for quote fetching process
These additions provide a more comprehensive view of relayer performance, combining balance information with quote data for improved monitoring and analysis in the Sanguine project.
No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/promexporter/config/config.go (3 hunks)
Additional context used
GitHub Check: Lint (contrib/promexporter)
contrib/promexporter/config/config.go
[failure] 26-26:
tag is not aligned , should be: default:"https://rpc.omnirpc.io" yaml:"omnirpc_url" (tagalign)
[failure] 36-36:
tag is not aligned , should be: default:"http://rfq-api.omnirpc.io/quotes" yaml:"rfq_api_url" (tagalign)
Additional comments not posted (1)
contrib/promexporter/config/config.go (1)
6-8
: LGTM! Import order improved.The reorganization of import statements enhances readability.
@@ -21,7 +22,8 @@ | |||
DFKPending []DFKPending `yaml:"dfk_pending"` | |||
// SubmitterChecks is the list of gas checks | |||
SubmitterChecks []SubmitterChecks `yaml:"gas_checks"` | |||
OmnirpcURL string `yaml:"omnirpc_url" default:"https://rpc.omnirpc.io"` | |||
// OmniRpcURL is the url of the omnirpc | |||
OmnirpcURL string `yaml:"omnirpc_url" default:"https://rpc.omnirpc.io"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align struct tags for consistency.
The OmnirpcURL
struct tag is not aligned properly. Aligning it improves readability and consistency.
- OmnirpcURL string `yaml:"omnirpc_url" default:"https://rpc.omnirpc.io"`
+ OmnirpcURL string `default:"https://rpc.omnirpc.io" yaml:"omnirpc_url"`
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
OmnirpcURL string `yaml:"omnirpc_url" default:"https://rpc.omnirpc.io"` | |
OmnirpcURL string `default:"https://rpc.omnirpc.io" yaml:"omnirpc_url"` |
Tools
GitHub Check: Lint (contrib/promexporter)
[failure] 26-26:
tag is not aligned , should be: default:"https://rpc.omnirpc.io" yaml:"omnirpc_url" (tagalign)
@@ -30,6 +32,8 @@ | |||
BridgeConfig BridgeConfig | |||
// BatchCallLimit is the limit of batch calls | |||
BatchCallLimit int | |||
// RFQAPIURL is the url of the RFQ API | |||
RFQAPIUrl string `yaml:"rfq_api_url" default:"http://rfq-api.omnirpc.io/quotes"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align struct tags for consistency.
The RFQAPIUrl
struct tag is not aligned properly. Aligning it improves readability and consistency.
- RFQAPIUrl string `yaml:"rfq_api_url" default:"http://rfq-api.omnirpc.io/quotes"`
+ RFQAPIUrl string `default:"http://rfq-api.omnirpc.io/quotes" yaml:"rfq_api_url"`
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
RFQAPIUrl string `yaml:"rfq_api_url" default:"http://rfq-api.omnirpc.io/quotes"` | |
RFQAPIUrl string `default:"http://rfq-api.omnirpc.io/quotes" yaml:"rfq_api_url"` |
Tools
GitHub Check: Lint (contrib/promexporter)
[failure] 36-36:
tag is not aligned , should be: default:"http://rfq-api.omnirpc.io/quotes" yaml:"rfq_api_url" (tagalign)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
Based on the provided information and the changes in the files, here's a summary of the key updates in this pull request:
Enhancements to Prometheus exporter for relayer metrics:
- Added functionality in
contrib/promexporter/exporters/relayer.go
to fetch relayer balances from multiple chains. - Implemented
fetchRelayerBalances
function to retrieve and process relayer addresses and balances. - Integrated relayer balance fetching into the
collectMetrics
function incontrib/promexporter/exporters/exporter.go
. - Extended the
otelRecorder
incontrib/promexporter/exporters/otel.go
to support recording and observing relayer balance metrics. - Added
RecordRelayerBalance
andrecordRelayerBalance
methods to handle relayer balance data.
These changes enhance the Prometheus exporter's capabilities to monitor and analyze relayer performance across different blockchain networks in the Sanguine project.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
This pull request continues to enhance the Prometheus exporter for relayer metrics, focusing on minor interface updates and data model improvements.
- Updated
iOtelRecorder
interface incontrib/promexporter/exporters/otel_generated.go
for cleaner relayer balance recording - Added 'Pool' field to Pet struct and PetFilter in
contrib/promexporter/internal/gql/dfk/models.gen.go
, expanding GraphQL API capabilities - Improved comments and parameter naming for better code clarity in relayer-related methods
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
// RELAYER CODE. | ||
RecordRelayerBalance(chainID int, relayer relayerMetadata) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using a more descriptive comment like '// Relayer Metrics.' for consistency with other method comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- contrib/promexporter/config/config.go (1 hunks)
- contrib/promexporter/exporters/otel_generated.go (1 hunks)
- contrib/promexporter/internal/gql/dfk/models.gen.go (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- contrib/promexporter/config/config.go
- contrib/promexporter/exporters/otel_generated.go
Additional comments not posted (4)
contrib/promexporter/internal/gql/dfk/models.gen.go (4)
2371-2371
: Addition ofPool
field inPet
struct looks good.The
Pool
field is optional and correctly serialized aspool
. This addition enhances thePet
struct by associating pets with a specific pool.
2701-2708
: Enhancements toPetFilter
struct are well-implemented.The new fields related to
Pool
provide comprehensive filtering options, enhancing the struct's capability to filter pets based on pool values.
4107-4107
: Addition ofPetOrderByPool
constant is appropriate.The new constant
PetOrderByPool
allows for ordering pets by their pool, aligning with the existing ordering options.
4138-4143
: Inclusion ofPetOrderByPool
inAllPetOrderBy
andIsValid
method is correct.The updates ensure that the new ordering option is recognized and validated properly, maintaining consistency across the codebase.
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
New Features
Bug Fixes
Chores